Skip to content

Fix: Sanitize SVG to prevent Stored XSS (Fixes #5514)#5515

Open
jeongbeannnn wants to merge 5 commits intogoogle:mainfrom
jeongbeannnn:fix/svg-xss-sanitization
Open

Fix: Sanitize SVG to prevent Stored XSS (Fixes #5514)#5515
jeongbeannnn wants to merge 5 commits intogoogle:mainfrom
jeongbeannnn:fix/svg-xss-sanitization

Conversation

@jeongbeannnn
Copy link
Copy Markdown

Summary

This PR fixes a Stored XSS vulnerability in the ADK web development UI that allows arbitrary JavaScript execution via malicious SVG artifacts (Issue #5514).

Changes

  • Added _sanitize_svg_content() function to remove XSS vectors from SVG content
  • Modified load_artifact() to sanitize SVG artifacts before returning to clients
  • Removes dangerous elements: <foreignObject>, <script> tags, and event handler attributes
  • Preserves legitimate SVG structure and valid attributes

Technical Details

Vulnerability

The vulnerability exists because the web dev UI uses Angular's bypassSecurityTrustHtml() to render SVG content without prior sanitization. An attacker can craft a malicious SVG artifact containing:

  • <foreignObject> elements with embedded XHTML
  • Event handlers like onerror, onload, onclick
  • JavaScript URLs in attributes

When a developer views the artifact in the dev UI, the XSS payload executes with full access to the developer's credentials and session.

Fix Strategy

Server-side SVG sanitization using regex-based filtering:

  1. Remove <foreignObject> elements completely
  2. Remove <script> tags
  3. Strip event handler attributes (onerror, onload, onclick, etc.)
  4. Remove javascript: URLs
  5. Preserve legitimate SVG structure and styling

Testing

  • Added comprehensive unit tests in tests/unittests/cli/test_svg_sanitization.py
  • 16 test cases covering:
    • foreignObject removal
    • script tag removal
    • Event handler removal (onload, onclick, onerror, onmouseover, etc.)
    • Valid SVG element preservation
    • SVG attribute preservation
    • JavaScript URL removal
    • Case-insensitive removal
    • Complex XSS payloads from issue Stored XSS via SVG foreignObject in ADK Web Dev UI #5514

Verification

  • Docker verification: Successfully tested against multiple XSS payloads
    • foreignObject + onerror handler ✓
    • Direct <script> tags ✓
    • Event handlers ✓
    • Valid SVG rendering still works ✓

References

@google-cla
Copy link
Copy Markdown

google-cla Bot commented Apr 28, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@adk-bot adk-bot added the web [Component] This issue will be transferred to adk-web label Apr 28, 2026
- Add _sanitize_svg_content() function to remove XSS vectors
- Sanitize SVG artifacts before returning from load_artifact()
- Remove foreignObject, script tags, and event handler attributes
- Add comprehensive unit tests for XSS prevention
- Verified with Docker v1.30.0 test cases
@jeongbeannnn jeongbeannnn force-pushed the fix/svg-xss-sanitization branch from c219054 to 0509ef2 Compare April 28, 2026 03:20
@jeongbeannnn jeongbeannnn marked this pull request as draft April 28, 2026 08:43
@jeongbeannnn jeongbeannnn marked this pull request as ready for review April 28, 2026 08:43
@jeongbeannnn jeongbeannnn marked this pull request as draft April 28, 2026 08:43
@jeongbeannnn jeongbeannnn marked this pull request as ready for review April 28, 2026 09:48
@jeongbeannnn jeongbeannnn marked this pull request as draft April 28, 2026 09:51
@jeongbeannnn jeongbeannnn marked this pull request as ready for review April 28, 2026 09:51
@rohityan rohityan self-assigned this Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

web [Component] This issue will be transferred to adk-web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants